Skip to content

Fix copy buttons on non-secure origins; move API keys into the sidebar - #1022

Merged
RhysSullivan merged 2 commits into
mainfrom
fix/api-keys-copy-and-sidebar
Jun 14, 2026
Merged

Fix copy buttons on non-secure origins; move API keys into the sidebar#1022
RhysSullivan merged 2 commits into
mainfrom
fix/api-keys-copy-and-sidebar

Conversation

@RhysSullivan

@RhysSullivan RhysSullivan commented Jun 14, 2026

Copy link
Copy Markdown
Collaborator

Feedback from a self-hosted user about API keys, plus a follow-up on copy failure handling.

1. The copy buttons didn't work

CopyButton (and the code blocks / the app update card) called
navigator.clipboard.writeText with no fallback. navigator.clipboard only
exists in a secure context (HTTPS or localhost), so on a self-hosted
console served over plain HTTP on a LAN host/IP the property is undefined
the click threw and was swallowed by void, so copy silently did nothing.

Fix: a shared copyToClipboard helper that tries the async Clipboard API and
falls back to selecting a hidden node via a document Range +
execCommand("copy"). The Range path deliberately does not move focus, so
it survives the focus trap of the dialog that shows a freshly-created key —
focusing a throwaway <textarea> there gets yanked straight back and copies
nothing (a real bug the e2e test caught). Every CopyButton, the code blocks,
and the app update card now route through the helper.

2. API keys wasn't in the sidebar

The link lived only in the account dropdown. It's now a first-class item in the
main sidebar nav for the hosts that serve keys in-app (self-host + cloud), and
removed from the dropdown (it's a move, not a duplicate). Cloudflare manages API
keys in Access, so it keeps the default nav without the item.

3. Failed copies now tell the user

If a copy genuinely can't happen (no navigator.clipboard and
execCommand refuses), the helper returns false and the button shows a
Failed to copy to clipboard error toast instead of failing silently.

Evidence

Three selfhost browser e2e scenarios (e2e/selfhost/api-keys-feedback.test.ts)
guard the behaviour — they fail on the old code and pass on this branch.

The copy button copies a new key on a (simulated) plain-HTTP origin:

API keys · the copy button copies a new key on a plain-HTTP self-host (selfhost)

API keys is reachable from the main sidebar:

API keys · the page is reachable from the main sidebar (selfhost)

A copy that can't reach the clipboard surfaces an error toast:

API keys · a copy that can't reach the clipboard surfaces an error toast (selfhost)

Gates green: format:check, lint, typecheck (39/39), packages/react unit
tests (154/154), and all three e2e scenarios.

The copy buttons (API key, MCP install command, tool IDs, …) called
navigator.clipboard.writeText with no fallback. navigator.clipboard only
exists in a secure context (HTTPS or localhost), so on a self-hosted console
served over plain HTTP on a LAN host/IP the property is undefined and the
click threw and was swallowed — copy silently did nothing.

Add a shared copyToClipboard helper (@executor-js/react lib/clipboard) that
tries the async Clipboard API and falls back to selecting a hidden node via a
document Range + execCommand("copy"). The Range path does NOT move focus, so
it still works inside a focus trap such as the dialog that shows a freshly
created API key (focusing a throwaway <textarea> there gets yanked straight
back, copying nothing). Route CopyButton, the code blocks, and the app update
card through it.

Move the API keys link out of the account dropdown and into the main sidebar
nav for the hosts that serve keys in-app (self-host + cloud). Cloudflare
manages API keys in Access, so it keeps the default nav without the item.

Add selfhost browser e2e guards for both: API keys is a first-class sidebar
item, and the copy button copies a new key on a simulated plain-HTTP origin.
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 14, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
executor-marketing 5f56924 Commit Preview URL

Branch Preview URL
Jun 14 2026, 10:54 PM

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 14, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
executor-cloud 5f56924 Jun 14 2026, 10:54 PM

@github-actions

github-actions Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Cloudflare preview

Torn down — the PR is closed.

@pkg-pr-new

pkg-pr-new Bot commented Jun 14, 2026

Copy link
Copy Markdown

Open in StackBlitz

@executor-js/cli

npm i https://pkg.pr.new/@executor-js/cli@1022

@executor-js/config

npm i https://pkg.pr.new/@executor-js/config@1022

@executor-js/execution

npm i https://pkg.pr.new/@executor-js/execution@1022

@executor-js/sdk

npm i https://pkg.pr.new/@executor-js/sdk@1022

@executor-js/codemode-core

npm i https://pkg.pr.new/@executor-js/codemode-core@1022

@executor-js/runtime-quickjs

npm i https://pkg.pr.new/@executor-js/runtime-quickjs@1022

@executor-js/plugin-file-secrets

npm i https://pkg.pr.new/@executor-js/plugin-file-secrets@1022

@executor-js/plugin-graphql

npm i https://pkg.pr.new/@executor-js/plugin-graphql@1022

@executor-js/plugin-keychain

npm i https://pkg.pr.new/@executor-js/plugin-keychain@1022

@executor-js/plugin-mcp

npm i https://pkg.pr.new/@executor-js/plugin-mcp@1022

@executor-js/plugin-onepassword

npm i https://pkg.pr.new/@executor-js/plugin-onepassword@1022

@executor-js/plugin-openapi

npm i https://pkg.pr.new/@executor-js/plugin-openapi@1022

executor

npm i https://pkg.pr.new/executor@1022

commit: 3fbfe30

@greptile-apps

greptile-apps Bot commented Jun 14, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes two self-hosted user-facing regressions: copy buttons silently doing nothing on plain-HTTP origins (where navigator.clipboard is undefined), and the API keys page being buried in the account dropdown instead of the main sidebar. A shared copyToClipboard helper adds a Range-based execCommand fallback that deliberately avoids moving focus, so it survives Radix dialog focus traps. Both fixes are guarded by new selfhost browser e2e scenarios.

  • packages/react/src/lib/clipboard.ts — new copyToClipboard helper tries the async Clipboard API and falls back to a hidden-span Range selection + execCommand("copy") that works without focus changes; all four call sites (CopyButton, CodeBlock, ExpandableCodeBlock, UpdateCard) are updated to handle the boolean return and show an error toast on failure.
  • Sidebar nav — self-host and cloud hosts now explicitly append { to: "/api-keys", label: "API keys" } to their nav item arrays; the Cloudflare host (which uses defaultShellNavItems as-is) correctly omits it since Access manages API keys there; the old apiKeysTo prop on ShellProps is removed entirely.
  • e2e/selfhost/api-keys-feedback.test.ts — three scenarios cover: (1) API keys reachable from the sidebar, (2) copy button works on a simulated non-secure origin via the execCommand fallback, (3) a fully blocked copy surfaces an error toast instead of silent failure.

Confidence Score: 5/5

Safe to merge — the changes are narrowly scoped to the clipboard helper, its four call sites, and the per-host sidebar nav arrays.

The clipboard helper is well-designed: the Range-based fallback avoids focus-trap issues documented in the PR, the hidden span is always cleaned up in a finally block, and the boolean return surface is handled uniformly at every call site with an error toast. The nav refactor removes the apiKeysTo prop cleanly and each host's nav array is explicit about what it includes. Three e2e scenarios cover the primary regression paths and the error case, and all format/lint/type/unit gates are green.

No files require special attention.

Important Files Changed

Filename Overview
packages/react/src/lib/clipboard.ts New clipboard utility — Range-based legacyCopy avoids focus-trap issues; node cleanup is in finally; returns boolean so callers surface failures correctly.
packages/react/src/multiplayer/shell.tsx Removes apiKeysTo prop and the inline API-keys dropdown item; API keys link is now a nav item added per-host instead.
apps/host-selfhost/web/routes/__root.tsx Adds /api-keys and /admin to the selfhost-specific nav item array.
apps/cloud/src/web/shell.tsx Adds /api-keys nav item after the shared defaults (minus /secrets) and before the cloud-only org/billing items; removes the now-deleted apiKeysTo prop.
apps/host-cloudflare/web/routes/__root.tsx Drops the explicit apiKeysTo={null} prop; Cloudflare still passes defaultShellNavItems which has no /api-keys entry, so behavior is unchanged.
e2e/selfhost/api-keys-feedback.test.ts Three selfhost browser scenarios guard both pieces of feedback; navigator.clipboard is patched out to simulate a non-secure origin; cleanup uses Effect.ensuring correctly.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User clicks Copy button] --> B{navigator.clipboard available?}
    B -->|Yes| C[await clipboard.writeText]
    C -->|Success| D[return true]
    C -->|Rejects| E[fall through to legacyCopy]
    B -->|No| E
    E --> F[Create hidden span with text]
    F --> G[Append span to document.body]
    G --> H[Save current Selection range]
    H --> I[Select span via Range - no focus move]
    I --> J[execCommand copy]
    J -->|true| K[return true]
    J -->|false or throws| L[return false]
    K --> M[finally: restore selection and remove span]
    L --> M
    D --> N{ok?}
    M --> N
    N -->|true| O[Show Copied confirmation]
    N -->|false| P[toast.error: Failed to copy]
Loading

Reviews (2): Last reviewed commit: "Toast on copy failure instead of failing..." | Re-trigger Greptile

Comment on lines +14 to +25
if (typeof navigator !== "undefined" && navigator.clipboard?.writeText) {
// oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: the Clipboard API rejects (permission denied, blur) and we recover via the legacy path
try {
await navigator.clipboard.writeText(text);
return true;
} catch {
// Permission denied / document not focused — fall through to the legacy
// path rather than failing the copy.
}
}

return legacyCopy(text);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Async gap may break execCommand user-gesture tracking on the fallback path. When the Clipboard API is present but rejects (e.g. permission denied on HTTPS), the await inside the try block crosses a microtask boundary before legacyCopy is called. Some browsers enforce that document.execCommand("copy") must be called synchronously within the original user-gesture task, so on those browsers the fallback silently does nothing — the same outcome as before this fix. The target scenario (non-secure origin where navigator.clipboard is undefined) is unaffected since it never enters the if block and calls legacyCopy synchronously.

Suggested change
if (typeof navigator !== "undefined" && navigator.clipboard?.writeText) {
// oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: the Clipboard API rejects (permission denied, blur) and we recover via the legacy path
try {
await navigator.clipboard.writeText(text);
return true;
} catch {
// Permission denied / document not focused — fall through to the legacy
// path rather than failing the copy.
}
}
return legacyCopy(text);
if (typeof navigator !== "undefined" && navigator.clipboard?.writeText) {
// oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: the Clipboard API rejects (permission denied, blur); falling through to legacyCopy after an await crosses a microtask boundary so we just report failure instead
try {
await navigator.clipboard.writeText(text);
return true;
} catch {
// Permission denied / document not focused. The async gap means
// legacyCopy may not have a live user-gesture token here.
return false;
}
}
return legacyCopy(text);

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

When a copy genuinely can't happen (no navigator.clipboard AND execCommand
refuses), copyToClipboard returns false. Show a "Failed to copy to clipboard"
error toast on that path across every copy site (CopyButton, the code blocks,
the app update card) rather than doing nothing.

Add a selfhost e2e guard that forces both copy paths to fail and asserts the
toast surfaces.
@RhysSullivan
RhysSullivan merged commit 498e4a6 into main Jun 14, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant